Skip to content

net/http: attempt deadlock fix in TestDisableKeepAliveUpgrade #43086

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

net/http: attempt deadlock fix in TestDisableKeepAliveUpgrade #43086

wants to merge 1 commit into from

Conversation

nhooyr
Copy link
Contributor

@nhooyr nhooyr commented Dec 9, 2020 •
edited
Loading

  1. The test now checks the response status code.
  2. The transport has been changed to not set "Connection: Close" if
    DisableKeepAlive is set and the request is a HTTP/1.1 protocol
    upgrade.

Updates #43073

@google-cla google-cla bot added the cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change. label Dec 9, 2020
@nhooyr nhooyr mentioned this pull request Dec 9, 2020
Closed
@nhooyr nhooyr force-pushed the fix-deadlock-92ad branch 2 times, most recently from e31f859 to fd783ef Compare December 9, 2020 01:21
@gopherbot
Copy link
Contributor

This PR (HEAD: fd783ef) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/276375 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@nhooyr nhooyr force-pushed the fix-deadlock-92ad branch 2 times, most recently from d32c154 to 4c05ef3 Compare December 10, 2020 20:04
@gopherbot
Copy link
Contributor

This PR (HEAD: 4c05ef3) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/276375 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Damien Neil:

Patch Set 3: Code-Review+2

Please don’t reply on this GitHub thread. Visit golang.org/cl/276375.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Dmitri Shuralyov:

Patch Set 3: Run-TryBot+1 Trust+1

(3 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/276375.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 3:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=dc3b6989

Please don’t reply on this GitHub thread. Visit golang.org/cl/276375.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 3:

Build is still in progress...
This change failed on freebsd-amd64-12_0:
See https://storage.googleapis.com/go-build-log/dc3b6989/freebsd-amd64-12_0_bbeba776.log

Other builds still in progress; subsequent failure notices suppressed until final report. Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.

Please don’t reply on this GitHub thread. Visit golang.org/cl/276375.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 3: TryBot-Result-1

20 of 21 TryBots failed:
Failed on freebsd-amd64-12_0: https://storage.googleapis.com/go-build-log/dc3b6989/freebsd-amd64-12_0_bbeba776.log
Failed on misc-compile-darwin: https://storage.googleapis.com/go-build-log/dc3b6989/misc-compile-darwin_13afe0f4.log
Failed on linux-amd64: https://storage.googleapis.com/go-build-log/dc3b6989/linux-amd64_8ac17cb5.log
Failed on misc-compile-darwinarm64: https://storage.googleapis.com/go-build-log/dc3b6989/misc-compile-darwinarm64_b328885b.log
Failed on linux-386: https://storage.googleapis.com/go-build-log/dc3b6989/linux-386_39944928.log
Failed on windows-amd64-2016: https://storage.googleapis.com/go-build-log/dc3b6989/windows-amd64-2016_bef08c52.log
Failed on windows-386-2008: https://storage.googleapis.com/go-build-log/dc3b6989/windows-386-2008_6d598353.log
Failed on openbsd-amd64-64: https://storage.googleapis.com/go-build-log/dc3b6989/openbsd-amd64-64_c4325c30.log
Failed on js-wasm: https://storage.googleapis.com/go-build-log/dc3b6989/js-wasm_3bd71e94.log
Failed on android-amd64-emu: https://storage.googleapis.com/go-build-log/dc3b6989/android-amd64-emu_a3ce2c92.log
Failed on linux-amd64-race: https://storage.googleapis.com/go-build-log/dc3b6989/linux-amd64-race_034e9bbc.log
Failed on misc-compile-solaris: https://storage.googleapis.com/go-build-log/dc3b6989/misc-compile-solaris_16bbf5aa.log
Failed on misc-compile-linuxarm: https://storage.googleapis.com/go-build-log/dc3b6989/misc-compile-linuxarm_ce95cd19.log
Failed on misc-compile-plan9: https://storage.googleapis.com/go-build-log/dc3b6989/misc-compile-plan9_51bb1484.log
Failed on misc-compile-freebsd: https://storage.googleapis.com/go-build-log/dc3b6989/misc-compile-freebsd_59727e85.log
Failed on misc-compile-ppc: https://storage.googleapis.com/go-build-log/dc3b6989/misc-compile-ppc_26459e7d.log
Failed on misc-compile-mips: https://storage.googleapis.com/go-build-log/dc3b6989/misc-compile-mips_15bd193d.log
Failed on misc-compile-netbsd: https://storage.googleapis.com/go-build-log/dc3b6989/misc-compile-netbsd_11ba7564.log
Failed on misc-compile-other: https://storage.googleapis.com/go-build-log/dc3b6989/misc-compile-other_1040a97a.log
Failed on misc-compile-openbsd: https://storage.googleapis.com/go-build-log/dc3b6989/misc-compile-openbsd_b4f1b440.log

Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.

Please don’t reply on this GitHub thread. Visit golang.org/cl/276375.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR (HEAD: e58bd9a) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/276375 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Anmol Sethi:

Patch Set 4:

(3 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/276375.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR (HEAD: fa61250) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/276375 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@nhooyr
net/http: attempt deadlock fix in TestDisableKeepAliveUpgrade
f809ceb 
1. The test now checks the response status code.
2. The transport has been changed to not set "Connection: Close" if
   DisableKeepAlive is set and the request is a HTTP/1.1 protocol
   upgrade.

Updates golang#43073
@gopherbot
Copy link
Contributor

This PR (HEAD: f809ceb) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/276375 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Bryan C. Mills:

Patch Set 6: Run-TryBot+1

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/276375.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 6:

SlowBots beginning. Status page: https://farmer.golang.org/try?commit=1a49556a

Please don’t reply on this GitHub thread. Visit golang.org/cl/276375.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 6: TryBot-Result+1

SlowBots are happy.

SlowBot builds that ran:

  • windows-amd64-longtest

Please don’t reply on this GitHub thread. Visit golang.org/cl/276375.
After addressing review feedback, remember to publish your drafts!

gopherbot pushed a commit that referenced this pull request Dec 14, 2020
@nhooyr @neild
net/http: attempt deadlock fix in TestDisableKeepAliveUpgrade
c81343c 
1. The test now checks the response status code.
2. The transport has been changed to not set "Connection: Close" if
   DisableKeepAlive is set and the request is a HTTP/1.1 protocol
   upgrade.

Updates #43073

Change-Id: I9977a18b33b8747ef847a8d11bb7b4f2d8053b8c
GitHub-Last-Rev: f809ceb
GitHub-Pull-Request: #43086
Reviewed-on: https://go-review.googlesource.com/c/go/+/276375
Run-TryBot: Bryan C. Mills <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Damien Neil <[email protected]>
Trust: Dmitri Shuralyov <[email protected]>
@gopherbot
Copy link
Contributor

This PR is being closed because golang.org/cl/276375 has been merged.

@gopherbot gopherbot closed this Dec 14, 2020
@nhooyr nhooyr deleted the fix-deadlock-92ad branch December 14, 2020 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@nhooyr @gopherbot